Corrections in docs, auth and database subscription bug fixes for iOS/Android#284
Open
abcd-ca wants to merge 7 commits intofullstackreact:v3from
Open
Corrections in docs, auth and database subscription bug fixes for iOS/Android#284abcd-ca wants to merge 7 commits intofullstackreact:v3from
abcd-ca wants to merge 7 commits intofullstackreact:v3from
Conversation
A couple mistakes in the docs that I tracked down in terms of keys and values for credentials.
Changed from facebook and google to facebook.com and google.com respectively. This is needed for cross platform apps. These strings are now consistent with the [iOS](https://github.com/fullstackreact/react-native-firestack/blob/v3/ios/Firestack/FirestackAuth.m#L444) and [Web](https://firebase.google.com/docs/reference/js/firebase.auth.AuthCredential) code. Docs should be updated too. [Here's my pull request](https://github.com/fullstackreact/react-native-firestack/pull/284/files) for that.
Solution discovered by @TheNarie in [this issue](fullstackreact#282) except I also corrected `accessToken` from the original suggestion. Docs also updated about that in same pull request.
The iOS and Android, `signInWithProvider` have different signatures. This works around that difference but ideally the native signatures should match each other (and the Web sdk's). Tested on iOS and Android.
needed for previous commit.
One of the needed parameters was named incorrectly – on Android this resulted in no data being retrieved. This fixes it.
Author
|
created issue to reference this PR about |
abcd-ca
changed the title
v1kku
reviewed
Feb 16, 2017
lib/modules/auth/index.js
Outdated
| */ | ||
| signInWithCredential(credential: any): Promise<Object> { | ||
| return promisify('signInWithProvider', FirestackAuth)(credential.provider, credential.accessToken, credential.secret); | ||
| signInWithCredential(credential: any): Promise<Object> { |
Author
|
Good catch, I fixed the duplicate line.
… On Feb 16, 2017, at 5:35 AM, Victor Munene ***@***.***> wrote:
@v1kku commented on this pull request.
In lib/modules/auth/index.js <#284 (comment)>:
> @@ -151,7 +151,12 @@ export default class Auth extends Base {
* @return {Promise} A promise resolved upon completion
*/
signInWithCredential(credential: any): Promise<Object> {
- return promisify('signInWithProvider', FirestackAuth)(credential.provider, credential.accessToken, credential.secret);
+ signInWithCredential(credential: any): Promise<Object> {
@abcd-ca <https://github.com/abcd-ca> This line is duplicated.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub <#284 (review)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABPG5ysoPvPP0iQYRn6lsYVN6YEMlWUTks5rdFCtgaJpZM4L7SE_>.
|
|
+1 |
1 similar comment
|
+1 |
Collaborator
|
@abcd-ca @alexkendall @JackThomson2 this repo has generally been abandoned, I think this issue and a ton of other issues were fixed in my v3 version of firestack. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A couple mistakes in the docs that I tracked down in terms of keys and values for credentials. I have facebook auth working in my app now as a result of these changes (I've verified)